Add `cosa import` by PeaceRebel · Pull Request #4164 · coreos/coreos-assembler

PeaceRebel · 2025-06-25T18:18:19Z

This command takes as argument a containers-transport(5)-style pullspec and creates a new cosa build dir from it. It essentially bridges the gap between coreos/fedora-coreos-config#3348 and the rest of the cosa pipeline.

co-authored by: Bipin B Narayan bbnaraya@redhat.com

openshift-ci · 2025-06-25T18:18:23Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

PeaceRebel · 2025-06-25T18:24:30Z

@jlebon I have pushed a very dirty draft here (using a new PR as I'm getting permission error when I try to push to your branch).

I'm getting error on cosa buildextend-qemu if this is the only build available and I'm looking at it. If there is a existing local build, I have to run the cosa buildextend-qemu twice to get a a bootable build.

I have set the ostree-content-checksum to a dummy value now. This should either come as a label for the container image or we should change it to be an optional field (this is set as a required field in the schema validation).

jlebon · 2025-06-26T16:19:43Z

I'm getting error on cosa buildextend-qemu if this is the only build available and I'm looking at it. If there is a existing local build, I have to run the cosa buildextend-qemu twice to get a a bootable build.

What error are you seeing?

I have set the ostree-content-checksum to a dummy value now. This should either come as a label for the container image or we should change it to be an optional field (this is set as a required field in the schema validation).

Yeah indeed, we should make it optional.

PeaceRebel · 2025-06-26T16:22:22Z

What error are you seeing?

This is the error if the import build is the only one available locally.

ERROR Opening ostree repository at '/srv/tmp/repo/../../cache/repo-build': opening repo: opendir(objects): No such file or directory
Traceback (most recent call last):
  File "<string>", line 10, in <module>
    cmdlib.import_ostree_commit(workdir, builddir, buildmeta, extract_json=('0' == '1'))
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/coreos-assembler/cosalib/cmdlib.py", line 337, in import_ostree_commit
    subprocess.check_call(['sudo', 'ostree', 'container', 'import', '--repo', build_repo,
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                           '--write-ref', buildmeta['buildid'],
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                           'ostree-unverified-image:oci-archive:' + tarfile])
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/subprocess.py", line 419, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', 'ostree', 'container', 'import', '--repo', '/srv/tmp/repo/../../cache/repo-build', '--write-ref', '43.20250614.91.0', 'ostree-unverified-image:oci-archive:/srv/builds/43.20250614.91.0/x86_64/fedora-coreos-43.20250614.91.0-ostree.x86_64.ociarchive']' returned non-zero exit status 1.
failed to execute cmd-build: exit status 1

PeaceRebel · 2025-06-27T12:22:34Z

What error are you seeing?

This is the error if the import build is the only one available locally.

ERROR Opening ostree repository at '/srv/tmp/repo/../../cache/repo-build': opening repo: opendir(objects): No such file or directory
Traceback (most recent call last):
  File "<string>", line 10, in <module>
    cmdlib.import_ostree_commit(workdir, builddir, buildmeta, extract_json=('0' == '1'))
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/coreos-assembler/cosalib/cmdlib.py", line 337, in import_ostree_commit
    subprocess.check_call(['sudo', 'ostree', 'container', 'import', '--repo', build_repo,
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                           '--write-ref', buildmeta['buildid'],
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                           'ostree-unverified-image:oci-archive:' + tarfile])
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/subprocess.py", line 419, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', 'ostree', 'container', 'import', '--repo', '/srv/tmp/repo/../../cache/repo-build', '--write-ref', '43.20250614.91.0', 'ostree-unverified-image:oci-archive:/srv/builds/43.20250614.91.0/x86_64/fedora-coreos-43.20250614.91.0-ostree.x86_64.ociarchive']' returned non-zero exit status 1.
failed to execute cmd-build: exit status 1

I found problem and fixed it now .

jlebon · 2025-06-27T13:14:15Z

I found problem and fixed it now .

For the record, the problem here was that tmp/repo was not initialized.

This is normally handled by prepare_build(), which is called by cosa build. There might be other things of that kind from prepare_build() that are still needed for e.g. a direct cosa buildextend-qemu to work.

It'd probably make sense to factor out those idempotent bits from prepare_build() into a separate function, and then have prepare_build() call that but also we can call it from cmd-osbuild

PeaceRebel · 2025-06-27T15:14:16Z

There might be other things of that kind from prepare_build() that are still needed for e.g. a direct cosa buildextend-qemu to work.

I'll take a look at that function, I skimmed through it to once. But with these changes I got a qemu image build, no errors.

PeaceRebel · 2025-07-02T08:23:19Z

@jlebon, I have removed the ref key from meta.json created here. I'm not sure what it's used for. It wasn't in the file for rhcos.

jlebon

Thanks for working on this! Did a first pass here but would like to test drive it as well. Did you sanity-checking that the kola tests pass with a QEMU image built from an imported build? There is cosa diff also that would be useful here, though that doesn't also cover e.g. the QEMU image. Though in practice, the input to the QEMU build is the container image, so a cosa diff at the ostree level should cover these. Still, worth also doing a cosa buildextend-live, and cosa diff with the various --live options too to sanity-check they're identical, and then do a kola testiso run.

jlebon · 2025-07-03T02:49:17Z

Make sure when testing this that you're testing with an FCOS built using https://github.com/coreos/fedora-coreos-config/blob/testing-devel/Containerfile and not what we currently push to https://quay.io/repository/fedora/fedora-coreos?tab=tags.

PeaceRebel · 2025-07-03T08:04:43Z

Make sure when testing this that you're testing with an FCOS built using https://github.com/coreos/fedora-coreos-config/blob/testing-devel/Containerfile and not what we currently push to https://quay.io/repository/fedora/fedora-coreos?tab=tags.

I was testing it with images from quay.io. I will check this.

cverna · 2025-07-04T11:15:43Z

Testing the gemini review integration

/gemini review

joelcapitao · 2025-07-04T13:22:17Z

You can now remove the commit updating the lockfiles (see #4171)

PeaceRebel · 2025-07-07T16:02:27Z

@jlebon I got qemu with the recent changes. Some of your PR comments are yet to be addressed. I'll be working on it soon.

PeaceRebel · 2025-07-16T13:01:03Z

osbuild patch is merged. osbuild/osbuild#2148

jlebon · 2025-07-16T15:32:58Z

Comments addressed! Also re-added the osbuild patch since it's merged upstream.

dustymabe

Thanks for working on this! Added some comments.

This command takes as argument a `containers-transport(5)`-style pullspec and creates a new cosa build dir from it. It essentially bridges the gap between coreos/fedora-coreos-config#3348 and the rest of the cosa pipeline. Co-authored-by: Jonathan Lebon <jonathan@jlebon.com>

There's a massive gotcha with bash, which is that even with `set -e`, if you're capturing output from a function, e.g. `x=$(myfunc)`, `set -e` will not be turned on in that function. That made an error in that function much harder to diagnose because the command _kept going_ even though the function failed (and then the overall operation failed later on on a much less clear error). There's a bash option to enable this (`shopt inherit_errexit`), but I think actually that in general we shouldn't be capturing output from non-trivial bash functions. It's hard in bash to ensure clean stdout output and the longer a function gets, the harder it is to keep this up (notice e.g. how that function needs to be careful to `echo` everything to stderr). In this case, it's easy to rework the flow here to avoid this, so let's do that instead.

We now keep blob refs in the tmp repo to make importing OCI images more efficient by avoiding having to re-import identical layers. But then we also need to prune them. Do this. The semantic here is simple: we prune any blob ref not tied to one of the kept builds.

Treefile.json from rpm-ostree doesn't contain `name` in the `metadata` field from images built using podman build.

jlebon · 2025-07-16T20:12:43Z

OK, addressed review comments! I ended up reworking this quite a bit in the process of wanting to clean up the data/metadata flow through the various functions. It looks very different, but it's really just shifting all the same pieces around to make it easier to follow.

Also fixed a minor bug in the process (we were missing a size key for the ostree artifact).

PeaceRebel · 2025-07-17T07:39:35Z

OK, addressed review comments! I ended up reworking this quite a bit in the process of wanting to clean up the data/metadata flow through the various functions. It looks very different, but it's really just shifting all the same pieces around to make it easier to follow.

Also fixed a minor bug in the process (we were missing a size key for the ostree artifact).

Thanks.

jbtrystram

Superficial LGTM as it touches some part i don't have experience with.
Thanks, that's nice !

Are we planning to have a cosa wrapper to do the container build using versionary to have a nicer build id like 20250717.dev.0 then import it ?

jlebon · 2025-07-17T14:36:36Z

So one bug here is that ostree-commit is not reproducible. That means that if you cosa import and then wipe tmp/repo and then e.g. cosa buildextend-qemu, when it tries to re-import the OCI archive, it might end up with a different digest and that throws things off.

Opened bootc-dev/bootc#1421 to fix this.

dustymabe

LGTM

I made some comments here but we can either disregard them or merge them in a later PR if you don't want to wait for a CI cycle on this.

dustymabe · 2025-07-17T20:15:46Z

+        if is_oci_imported:
+            import_oci_archive(tmpdir, tarfile, buildmeta['buildid'])


I think this part is still tripping me up a bit and I think it's the naming of things.

For example, we're calling checking is_oci_imported here and then calling import_oci_archive, but isn't the elif/else below also importing a .ociarchive file?

Maybe if we named the function import_container_built_ociarchive it would make more sense, because basically the different here that I see is in this case we are importing an archive that was created via podman build and in the other cases we are importing an archive that was generated by rpm-ostree.

I don't actually know if that rename suggestion is something we should do, just putting my thought process out there. Maybe just a comment would suffice.

Yeah, it should probably be called was_oci_imported.

dustymabe · 2025-07-17T20:16:57Z

-        if os.environ.get('COSA_PRIVILEGED', '') == '1':
+        if is_oci_imported:
+            import_oci_archive(tmpdir, tarfile, buildmeta['buildid'])
+        elif os.environ.get('COSA_PRIVILEGED', '') == '1':


not new to this PR but I don't really know why we need both an elif/else here for the legacy path based on COSA_PRIVILEGED. Would just using the unprivileged path work in both cases (just be slower)?

Yes it would. The privileged path takes advantage of the fact that we already have a bare-user repo laying around that probably has a majority of the objects already.

dustymabe · 2025-07-17T20:19:00Z

            extract_image_json(workdir, commit)


+def import_oci_archive(parent_tmpd, ociarchive, ref):


Comparing this to the legacy unpriv path above it seems like the biggest differences here are:

handling of the blob refs

we call ostree container image pull versus ostree container import is that right?

Correct, yes. The framing is quite different. With ostree container import, the source of truth is an OSTree commit that was exported to a tarball and we're reconverting it into the exact original OSTree commit. With ostree container image pull, the source of truth is the container image, and we're importing that into the tmp repo to mesh with existing tooling.

dustymabe · 2025-07-17T20:20:03Z

+                                            encoding='utf-8').splitlines()
+        subprocess.check_call(['ostree', 'pull-local', '--repo', 'tmp/repo', tmpd] + blob_refs)
+
+    return subprocess.check_output(['ostree', 'rev-parse', '--repo', 'tmp/repo', ref], encoding='utf-8').strip()


Might make things a little more obvious

Suggested change

return subprocess.check_output(['ostree', 'rev-parse', '--repo', 'tmp/repo', ref], encoding='utf-8').strip()

ostree_commit = subprocess.check_output(['ostree', 'rev-parse', '--repo', 'tmp/repo', ref], encoding='utf-8').strip()

return ostree_commit

dustymabe · 2025-07-17T20:20:38Z

@@ -354,6 +360,42 @@ def import_ostree_commit(workdir, buildpath, buildmeta, extract_json=True, parti
            extract_image_json(workdir, commit)




A comment here mentioning some callers expect the extracting ostree commit hash string to be returned might be useful.

jlebon

Thanks for the reviews! Let's get this in and do tweaks in follow-ups.

jlebon · 2025-07-18T01:14:22Z

+        if is_oci_imported:
+            import_oci_archive(tmpdir, tarfile, buildmeta['buildid'])


Yeah, it should probably be called was_oci_imported.

jlebon · 2025-07-18T01:47:04Z

-        if os.environ.get('COSA_PRIVILEGED', '') == '1':
+        if is_oci_imported:
+            import_oci_archive(tmpdir, tarfile, buildmeta['buildid'])
+        elif os.environ.get('COSA_PRIVILEGED', '') == '1':


Yes it would. The privileged path takes advantage of the fact that we already have a bare-user repo laying around that probably has a majority of the objects already.

jlebon · 2025-07-18T01:51:25Z

            extract_image_json(workdir, commit)


+def import_oci_archive(parent_tmpd, ociarchive, ref):


Correct, yes. The framing is quite different. With ostree container import, the source of truth is an OSTree commit that was exported to a tarball and we're reconverting it into the exact original OSTree commit. With ostree container image pull, the source of truth is the container image, and we're importing that into the tmp repo to mesh with existing tooling.

Address some comments from coreos#4164.

jlebon · 2025-07-18T13:35:03Z

Follow-ups in #4227.

Address some comments from coreos#4164.

Address some comments from #4164.

PeaceRebel self-assigned this Jun 25, 2025

openshift-ci Bot added the do-not-merge/work-in-progress label Jun 25, 2025

jbtrystram reviewed Jun 27, 2025

View reviewed changes

Comment thread cmd/coreos-assembler.go Outdated

PeaceRebel force-pushed the pr/cmd-import branch from ac369ad to 4f68cef Compare June 27, 2025 12:21

PeaceRebel force-pushed the pr/cmd-import branch 2 times, most recently from cd0f927 to b9d955d Compare July 1, 2025 17:47

PeaceRebel changed the title ~~WIP: Add cosa import~~ Add cosa import Jul 1, 2025

PeaceRebel marked this pull request as ready for review July 1, 2025 17:49

openshift-ci Bot removed the do-not-merge/work-in-progress label Jul 1, 2025

PeaceRebel requested a review from jlebon July 1, 2025 17:49

PeaceRebel force-pushed the pr/cmd-import branch from b9d955d to 069c813 Compare July 1, 2025 17:59

jlebon reviewed Jul 3, 2025

View reviewed changes

This was referenced Jul 4, 2025

WIP: Add cosa import #4137

Closed

Build FCOS based on Fedora 43 using podman build coreos/fedora-coreos-tracker#1969

Closed

openshift-merge-robot added the needs-rebase label Jul 4, 2025

PeaceRebel force-pushed the pr/cmd-import branch from deacbb7 to aed523c Compare July 7, 2025 15:59

openshift-merge-robot removed the needs-rebase label Jul 7, 2025

PeaceRebel mentioned this pull request Jul 16, 2025

org.osbuild.coreos.live-artifacts.mono: read os name from /usr/lib/os-release osbuild/osbuild#2148

Merged

jlebon dismissed their stale review via d0be01d July 16, 2025 15:32

jlebon force-pushed the pr/cmd-import branch from 7131be0 to d0be01d Compare July 16, 2025 15:32

PeaceRebel force-pushed the pr/cmd-import branch from d0be01d to 0ab5a02 Compare July 16, 2025 16:30

PeaceRebel requested a review from jbtrystram July 16, 2025 16:31

dustymabe reviewed Jul 16, 2025

View reviewed changes

Comment thread src/cmd-osbuild Outdated

Comment thread src/cmd-import Outdated

Comment thread src/cmd-import

Comment thread src/cmd-import Outdated

Comment thread src/cmd-import Outdated

Comment thread src/cmd-import Outdated

PeaceRebel and others added 4 commits July 16, 2025 16:07

Patch osbuild to generate osname from os-release file

dcefea3

Treefile.json from rpm-ostree doesn't contain `name` in the `metadata` field from images built using podman build.

jlebon force-pushed the pr/cmd-import branch from 0ab5a02 to dcefea3 Compare July 16, 2025 20:09

jbtrystram reviewed Jul 17, 2025

View reviewed changes

dustymabe approved these changes Jul 17, 2025

View reviewed changes

jlebon reviewed Jul 18, 2025

View reviewed changes

jlebon merged commit 7a6e3a0 into coreos:main Jul 18, 2025
5 checks passed

jlebon added a commit to jlebon/coreos-assembler that referenced this pull request Jul 18, 2025

cmdlib.py: minor tweaks related to OCI imports

f5d28ce

Address some comments from coreos#4164.

jlebon mentioned this pull request Jul 18, 2025

cmdlib.py: minor tweaks related to OCI imports #4227

Merged

jlebon added a commit to jlebon/coreos-assembler that referenced this pull request Jul 18, 2025

cmdlib.py: minor tweaks related to OCI imports

4563969

Address some comments from coreos#4164.

jlebon mentioned this pull request Jul 18, 2025

kola: ext.config.rpm-ostree.kernel-replace fails with "no space left on device" coreos/fedora-coreos-tracker#1984

Closed

dustymabe pushed a commit that referenced this pull request Jul 18, 2025

cmdlib.py: minor tweaks related to OCI imports

bf90d46

Address some comments from #4164.

cverna mentioned this pull request Jul 23, 2025

support cosa init --ostree docker://quay.io/coreos-assembler/fcos:testing-devel #2685

Closed

jbtrystram mentioned this pull request Jul 25, 2025

Teach jenkins build pipeline to import an existing OCI image coreos/fedora-coreos-tracker#1990

Closed

travier mentioned this pull request Jul 31, 2025

cosa import: Do not fail on non-existing com.coreos.osname label #4244

Closed

jlebon mentioned this pull request Aug 14, 2025

Leverage CoreOS test framework for our CI bootc-dev/bootc#1514

Open

		if is_oci_imported:
		import_oci_archive(tmpdir, tarfile, buildmeta['buildid'])

		extract_image_json(workdir, commit)


		def import_oci_archive(parent_tmpd, ociarchive, ref):

	return subprocess.check_output(['ostree', 'rev-parse', '--repo', 'tmp/repo', ref], encoding='utf-8').strip()
	ostree_commit = subprocess.check_output(['ostree', 'rev-parse', '--repo', 'tmp/repo', ref], encoding='utf-8').strip()
	return ostree_commit

		@@ -354,6 +360,42 @@ def import_ostree_commit(workdir, buildpath, buildmeta, extract_json=True, parti
		extract_image_json(workdir, commit)

Conversation

PeaceRebel commented Jun 25, 2025

Uh oh!

openshift-ci Bot commented Jun 25, 2025

Uh oh!

PeaceRebel commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jlebon commented Jun 26, 2025

Uh oh!

PeaceRebel commented Jun 26, 2025

Uh oh!

Uh oh!

PeaceRebel commented Jun 27, 2025

Uh oh!

jlebon commented Jun 27, 2025

Uh oh!

PeaceRebel commented Jun 27, 2025

Uh oh!

PeaceRebel commented Jul 2, 2025

Uh oh!

jlebon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jlebon commented Jul 3, 2025

Uh oh!

PeaceRebel commented Jul 3, 2025

Uh oh!

cverna commented Jul 4, 2025

Uh oh!

joelcapitao commented Jul 4, 2025

Uh oh!

PeaceRebel commented Jul 7, 2025

Uh oh!

PeaceRebel commented Jul 16, 2025

Uh oh!

jlebon commented Jul 16, 2025

Uh oh!

dustymabe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jlebon commented Jul 16, 2025

Uh oh!

PeaceRebel commented Jul 17, 2025

Uh oh!

jbtrystram left a comment

Choose a reason for hiding this comment

Uh oh!

jlebon commented Jul 17, 2025

Uh oh!

dustymabe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeaceRebel commented Jun 25, 2025 •

edited

Loading

jlebon left a comment •

edited

Loading